Expand returned context output of IP reputation command in IPInfo#43615
Expand returned context output of IP reputation command in IPInfo#43615TheL0L wants to merge 10 commits into
Conversation
TheL0L
added
python
🤖 AI-Powered Code Review AvailableYou can leverage AI-powered code review to assist with this PR! Available Commands:
|
…ipinfo-expand-returned-context
marketplace-ai-reviewer
removed
the
ready-for-ai-review
|
🤖 Analysis started. Please wait for results... |
🤖 AI Review DisclaimerThis review was generated by an AI-powered tool and may contain inaccuracies. Please be advised, and we extend our sincere apologies for any inconvenience this may cause. |
marketplace-ai-reviewer
left a comment
marketplace-ai-reviewer
left a comment
There was a problem hiding this comment.
Thanks for your contribution to the IPinfo integration! I've reviewed the updates and have a few suggestions to help align the code with our standards.
Primarily, please avoid dumping the raw API response directly into the context, and ensure that any newly exposed fields are properly documented in the YAML outputs. There are also a couple of minor cleanups needed, such as removing unnecessary # type: ignore comments and slightly adjusting the integration description for a more neutral tone.
Thanks again for your work on this!
Additionally, please address the following file-level notes:
Packs/ipinfo/Integrations/ipinfo_v2/ipinfo_v2.py: Update integration YAML with new context outputs.Packs/ipinfo/Integrations/ipinfo_v2/ipinfo_v2.yml: The PR description and Python code indicate that the context outputs have been extended to include the full API response (entry_context = raw_result.copy()). However, these new fields (e.g.,city,region,postal,timezone, etc.) are not documented in the YML file'soutputssection.
According to XSOAR guidelines, all command outputs must be defined in the YML file with appropriate descriptions. Please add the newly exposed fields to the outputs section of the ip command. If some fields are only available in specific plans, you can note that in their descriptions (similar to how IPinfo.IP.Organization.Name is documented).
@kamalq97 please review and approve the results generated by the AI Reviewer by responding 👍 on this comment.
| advanced: true | ||
| required: false | ||
| description: Use the IPinfo.io API to get data about an IP address. | ||
| description: 'Use the IPinfo.io API to get data about an IP address. We support all plan types: Lite, Plus, Core, Enterprise. The response from the API will match your plan type.' |
There was a problem hiding this comment.
Consider rephrasing "We support all plan types" to a more neutral, professional tone, such as "Supports all plan types".
description: 'Use the IPinfo.io API to get data about an IP address. Supports all plan types: Lite, Plus, Core, Enterprise. The response from the API will match your plan type.'
kamalq97
changed the title
|
Validate summary Verdict: PR can be force merged from validate perspective? ✅ |
kamalq97
left a comment
kamalq97
left a comment
There was a problem hiding this comment.
Great work!
Please address all my comments below.
Please consider mentioning multi-tier support in the ..._description.md file of the integration.
|
This PR is marked as 'Stale' because it has been open for 30 days with no activity, it will be automatically closed in 15 days if no activity will be done. To reset the counter just remove the 'Stale' label or make changes to update this PR. If you wish this PR will never be marked as 'Stale' add the 'Ignore Stale' |
6 participants
Related Issues
fixes: CIAC-15750
Description
Extend the context outputs to include full response.